Add IsEquivalentTo assertion for Memory<T> and ReadOnlyMemory<T>#5204
Add IsEquivalentTo assertion for Memory<T> and ReadOnlyMemory<T>#5204
Conversation
thomhurst
had a problem deploying
to
Pull Requests
GitHub Actions
Failure
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
Overall, this is a well-implemented addition that correctly extends to and . The PR follows existing patterns, correctly delegates to , handles AOT annotations, and includes good test coverage.
A few observations:
Issue: Missing .Using() fluent methods — API inconsistency
The IsEquivalentToAssertion<TCollection, TItem> for regular IEnumerable<T> exposes two fluent overrides:
public IsEquivalentToAssertion<TCollection, TItem> Using(IEqualityComparer<TItem> comparer) { ... }
public IsEquivalentToAssertion<TCollection, TItem> Using(Func<TItem?, TItem?, bool> equalityPredicate) { ... }The new MemoryIsEquivalentToAssertion<TMemory, TItem> lacks these methods entirely. This means users who want a fluent .Using() comparer on memory assertions must use the constructor overload — which breaks the consistency of the API and is a potential point of confusion since the same pattern works on collection assertions.
Suggested fix: Add the same .Using() methods to MemoryIsEquivalentToAssertion mirroring the collection version. This is especially important given the comparer is stored as a non-nullable IEqualityComparer<TItem> field, which implies mutation post-construction was anticipated.
Minor: Duplicated expression builder logic
Both IsEquivalentTo overloads in MemoryAssertionBase contain nearly-identical expression building blocks with an added flag pattern:
Context.ExpressionBuilder.Append(".IsEquivalentTo(");
var added = false;
if (expectedExpression != null) { ...; added = true; }
if (orderingExpression != null) { Context.ExpressionBuilder.Append(added ? ", " : ""); ... }
Context.ExpressionBuilder.Append(')');The second overload just adds a comparerExpression segment in the middle. This could be extracted to a small private helper method (e.g., AppendIsEquivalentToExpression) to reduce duplication and make future updates to expression formatting a single-place change.
Observation: metadata.Value == null is dead code for value types
TMemory is always Memory<T> or ReadOnlyMemory<T> — both structs — so metadata.Value == null can never be true. This null check is repeated across all other memory assertions in the file, so it's consistent with existing patterns, but worth noting it's unreachable.
Good
- Correct AOT annotation:
[RequiresUnreferencedCode]is applied only to the constructor that usesStructuralEqualityComparer(which requires reflection), and the matchingIsEquivalentTomethod overload. The custom-comparer path is correctly left unannotated. - Proper delegation to
CollectionEquivalencyChecker.AreEquivalent— no duplication of equivalency logic. - Public API snapshots updated for all three target frameworks.
- Test coverage is solid: unordered (default), ordered, different types (
int,byte), count mismatch, and value mismatch failure cases are all covered.
There was a problem hiding this comment.
Code Review
Overall, this is a well-implemented addition that correctly extends IsEquivalentTo to Memory<T> and ReadOnlyMemory<T>. The PR follows existing patterns, correctly delegates to CollectionEquivalencyChecker, handles AOT annotations, and includes good test coverage.
A few observations:
Issue: Missing .Using() fluent methods — API inconsistency
The IsEquivalentToAssertion<TCollection, TItem> for regular IEnumerable<T> exposes two fluent overrides:
public IsEquivalentToAssertion<TCollection, TItem> Using(IEqualityComparer<TItem> comparer) { ... }
public IsEquivalentToAssertion<TCollection, TItem> Using(Func<TItem?, TItem?, bool> equalityPredicate) { ... }The new MemoryIsEquivalentToAssertion<TMemory, TItem> lacks these methods. This means users who want fluent .Using() on memory assertions must use the constructor overload — breaking consistency with the collection assertion API. The comparer is even stored as a non-nullable IEqualityComparer<TItem> field, which suggests post-construction mutation was anticipated but not completed.
Suggested fix: Add the same .Using() methods to MemoryIsEquivalentToAssertion mirroring the collection version.
Minor: Duplicated expression builder logic
Both IsEquivalentTo overloads in MemoryAssertionBase contain nearly-identical expression building blocks with an added flag pattern. The second overload only adds a comparerExpression segment in the middle. This could be extracted to a small private helper to reduce duplication and make future formatting changes a single-place edit.
Observation: metadata.Value == null is dead code for value types
TMemory is always Memory<T> or ReadOnlyMemory<T> — both structs — so the null check can never be true. This is consistent with other memory assertions in the file, but it's unreachable.
Good
- Correct AOT annotation:
[RequiresUnreferencedCode]is applied only to the constructor that usesStructuralEqualityComparer(which requires reflection), and the matching method overload. The custom-comparer path is correctly left unannotated. - Proper delegation to
CollectionEquivalencyChecker.AreEquivalentwith no duplication of equivalency logic. - Public API snapshots updated for all three target frameworks.
- Test coverage is solid: unordered (default), ordered, different element types (
int,byte), count mismatch, and value mismatch failure cases are all covered.
[//]: # (dependabot-start)⚠️ **Dependabot is rebasing this PR**⚠️ Rebasing might not happen immediately, so don't worry if this takes some time. Note: if you make any changes to this PR yourself, they will take precedence over the rebase. --- [//]: # (dependabot-end) Updated [TUnit.Core](https://github.com/thomhurst/TUnit) from 1.19.57 to 1.21.6. <details> <summary>Release notes</summary> _Sourced from [TUnit.Core's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.21.6 <!-- Release notes generated using configuration in .github/release.yml at v1.21.6 --> ## What's Changed ### Other Changes * perf: replace object locks with Lock type for efficient synchronization by @thomhurst in thomhurst/TUnit#5219 * perf: parallelize test metadata collection for source-generated tests by @thomhurst in thomhurst/TUnit#5221 * perf: use GetOrAdd args overload to eliminate closure allocations in event receivers by @thomhurst in thomhurst/TUnit#5222 * perf: self-contained TestEntry<T> with consolidated switch invokers eliminates per-test JIT by @thomhurst in thomhurst/TUnit#5223 ### Dependencies * chore(deps): update tunit to 1.21.0 by @thomhurst in thomhurst/TUnit#5220 **Full Changelog**: thomhurst/TUnit@v1.21.0...v1.21.6 ## 1.21.0 <!-- Release notes generated using configuration in .github/release.yml at v1.21.0 --> ## What's Changed ### Other Changes * perf: reduce ConcurrentDictionary closure allocations in hot paths by @thomhurst in thomhurst/TUnit#5210 * perf: reduce async state machine overhead in test execution pipeline by @thomhurst in thomhurst/TUnit#5214 * perf: reduce allocations in EventReceiverOrchestrator and TestContextExtensions by @thomhurst in thomhurst/TUnit#5212 * perf: skip timeout machinery when no timeout configured by @thomhurst in thomhurst/TUnit#5211 * perf: reduce allocations and lock contention in ObjectTracker by @thomhurst in thomhurst/TUnit#5213 * Feat/numeric tolerance by @agray in thomhurst/TUnit#5110 * perf: remove unnecessary lock in ObjectTracker.TrackObjects by @thomhurst in thomhurst/TUnit#5217 * perf: eliminate async state machine in TestCoordinator.ExecuteTestAsync by @thomhurst in thomhurst/TUnit#5216 * perf: eliminate LINQ allocation in ObjectTracker.UntrackObjectsAsync by @thomhurst in thomhurst/TUnit#5215 * perf: consolidate module initializers into single .cctor via partial class by @thomhurst in thomhurst/TUnit#5218 ### Dependencies * chore(deps): update tunit to 1.20.0 by @thomhurst in thomhurst/TUnit#5205 * chore(deps): update dependency nunit3testadapter to 6.2.0 by @thomhurst in thomhurst/TUnit#5206 * chore(deps): update dependency cliwrap to 3.10.1 by @thomhurst in thomhurst/TUnit#5207 **Full Changelog**: thomhurst/TUnit@v1.20.0...v1.21.0 ## 1.20.0 <!-- Release notes generated using configuration in .github/release.yml at v1.20.0 --> ## What's Changed ### Other Changes * Fix inverted colors in HTML report ring chart due to locale-dependent decimal formatting by @Copilot in thomhurst/TUnit#5185 * Fix nullable warnings when using Member() on nullable properties by @Copilot in thomhurst/TUnit#5191 * Add CS8629 suppression and member access expression matching to IsNotNullAssertionSuppressor by @Copilot in thomhurst/TUnit#5201 * feat: add ConfigureAppHost hook to AspireFixture by @thomhurst in thomhurst/TUnit#5202 * Fix ConfigureTestConfiguration being invoked twice by @thomhurst in thomhurst/TUnit#5203 * Add IsEquivalentTo assertion for Memory<T> and ReadOnlyMemory<T> by @thomhurst in thomhurst/TUnit#5204 ### Dependencies * chore(deps): update dependency gitversion.tool to v6.6.2 by @thomhurst in thomhurst/TUnit#5181 * chore(deps): update dependency gitversion.msbuild to 6.6.2 by @thomhurst in thomhurst/TUnit#5180 * chore(deps): update tunit to 1.19.74 by @thomhurst in thomhurst/TUnit#5179 * chore(deps): update verify to 31.13.3 by @thomhurst in thomhurst/TUnit#5182 * chore(deps): update verify to 31.13.5 by @thomhurst in thomhurst/TUnit#5183 * chore(deps): update aspire to 13.1.3 by @thomhurst in thomhurst/TUnit#5189 * chore(deps): update dependency stackexchange.redis to 2.12.4 by @thomhurst in thomhurst/TUnit#5193 * chore(deps): update microsoft/setup-msbuild action to v3 by @thomhurst in thomhurst/TUnit#5197 **Full Changelog**: thomhurst/TUnit@v1.19.74...v1.20.0 ## 1.19.74 <!-- Release notes generated using configuration in .github/release.yml at v1.19.74 --> ## What's Changed ### Other Changes * feat: per-hook activity spans with method names by @thomhurst in thomhurst/TUnit#5159 * fix: add tooltip to truncated span names in HTML report by @thomhurst in thomhurst/TUnit#5164 * Use enum names instead of numeric values in test display names by @Copilot in thomhurst/TUnit#5178 * fix: resolve CS8920 when mocking interfaces whose members return static-abstract interfaces by @lucaxchaves in thomhurst/TUnit#5154 ### Dependencies * chore(deps): update tunit to 1.19.57 by @thomhurst in thomhurst/TUnit#5157 * chore(deps): update dependency gitversion.msbuild to 6.6.1 by @thomhurst in thomhurst/TUnit#5160 * chore(deps): update dependency gitversion.tool to v6.6.1 by @thomhurst in thomhurst/TUnit#5161 * chore(deps): update dependency polyfill to 9.20.0 by @thomhurst in thomhurst/TUnit#5163 * chore(deps): update dependency polyfill to 9.20.0 by @thomhurst in thomhurst/TUnit#5162 * chore(deps): update dependency polyfill to 9.21.0 by @thomhurst in thomhurst/TUnit#5166 * chore(deps): update dependency polyfill to 9.21.0 by @thomhurst in thomhurst/TUnit#5167 * chore(deps): update dependency polyfill to 9.22.0 by @thomhurst in thomhurst/TUnit#5168 * chore(deps): update dependency polyfill to 9.22.0 by @thomhurst in thomhurst/TUnit#5169 * chore(deps): update dependency coverlet.collector to 8.0.1 by @thomhurst in thomhurst/TUnit#5177 ## New Contributors * @lucaxchaves made their first contribution in thomhurst/TUnit#5154 **Full Changelog**: thomhurst/TUnit@v1.19.57...v1.19.74 Commits viewable in [compare view](thomhurst/TUnit@v1.19.57...v1.21.6). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
1 participant
Summary
IsEquivalentTomethods toMemoryAssertionBase<TMemory, TItem>, making them available on bothMemory<T>andReadOnlyMemory<T>assertionsCollectionOrdering, plus customIEqualityComparer<TItem>overloadCollectionEquivalencyCheckervia the adapter patternCloses #5171
Test plan
Memory<int>andReadOnlyMemory<byte>equivalency (ordered, unordered, failure cases)MemoryAssertionTestspass on net9.0